Pull request overview

This PR fixes a false-positive warning emitted by AzureAIClient when use_latest_version=True and the framework supplies an empty runtime tools list ([]), reducing log noise and user confusion.

Changes:

• Adjusted _remove_agent_level_run_options() to treat empty runtime tools as “no runtime tools supplied” when there is no creation-time baseline.
• Updated the warning string formatting (same message content, reflowed).
• Added a regression test covering the use_latest_version=True + empty tools scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

Automated Code Review

Reviewers: 4 | Confidence: 92%

✓ Correctness

The change fixes a false-positive warning when use_latest_version=True and the framework passes an empty runtime tools list. Previously, when warn_runtime_tools_and_structure_changed was False, any non-None runtime_tools (including []) would trigger a spurious mismatch warning. The new code introduces an else branch that uses a truthiness check (if runtime_tools:) so empty lists are ignored, while non-empty tool lists still correctly trigger the warning. The structured_output check correctly remains is not None since there is no analogous empty-default for that field. The test properly exercises the regression scenario with use_latest_version=True and empty tools. Existing tests for non-empty tools with warn_runtime_tools_and_structure_changed=False (e.g., test_prepare_options_logs_warning_for_tools_with_existing_agent_version) are unaffected because non-empty lists are still truthy.

✓ Security Reliability

This diff fixes a false-positive warning when use_latest_version=True causes the framework to pass an empty tools list ([]). Previously, the else branch (when warn_runtime_tools_and_structure_changed is False) unconditionally set tools_changed = True whenever runtime_tools is not None, which includes the empty-list case. The fix correctly initializes both flags to False and, in the else branch, uses a truthiness check (if runtime_tools:) to skip empty lists. The logic is correct and the test adequately covers the regression. No security, reliability, or resource-leak concerns were identified.

✗ Test Coverage

The diff fixes a false-positive warning when use_latest_version=True by distinguishing empty runtime tools ([]) from actual user-supplied tools. The new test (test_use_latest_version_no_spurious_warning_for_empty_tools) correctly exercises the else branch of _remove_agent_level_run_options with an empty tools list and asserts that no warning is emitted. However, there is no complementary test verifying that non-empty tools with use_latest_version=True still correctly trigger the warning — this is the positive case for the same else branch. Without it, a future regression that accidentally suppresses all warnings in the else branch would go undetected. Additionally, the runtime_structured_output path in the else branch remains untested when warn_runtime_tools_and_structure_changed is False.

✓ Design Approach

The fix correctly addresses a real false-positive warning: when use_latest_version=True (or an explicit agent_version is supplied), warn_runtime_tools_and_structure_changed stays False because the client never called create_version. Under the old code, an empty [] tools list passed by the framework was enough to set tools_changed = True (because [] is not None), firing a spurious warning. The new approach initializes both flags to False and uses truthiness (if runtime_tools:) in the no-baseline branch to ignore an empty list, which is the right semantic. The _get_structured_output_signature helper already normalises response_format so it returns None for missing values, meaning if runtime_structured_output is not None in the else-branch is reliable. The regression test correctly exercises the use_latest_version path through _prepare_options end-to-end (not just the helper), which gives confidence the fix holds in the real call stack. One minor inaccuracy in the new comment: when use_latest_version=True but the agent does not yet exist, the fallback creation path sets warn_runtime_tools_and_structure_changed = True, so that case does NOT end up in the else branch — the comment implies it always does. This is a docs-only concern and does not affect correctness.

Flagged Issues

• Missing complementary test: there is no test verifying that non-empty tools with use_latest_version=True (or explicit agent_version) still trigger the warning via the new else branch. The existing test_prepare_options_logs_warning_for_tools_with_existing_agent_version covers agent_version but was written against the old logic. A dedicated test for use_latest_version=True with non-empty tools is needed to guard against regressions that silently suppress all warnings in this code path.

Suggestions

• Add a test for runtime_structured_output is not None when warn_runtime_tools_and_structure_changed=False to cover the structured-output half of the new else branch.
• Tighten the inline comment on the else branch: when use_latest_version=True but the agent does not yet exist, the fallback creation path sets warn_runtime_tools_and_structure_changed = True, so this branch is never reached in that case. Consider wording like: 'Agent was fetched externally (use_latest_version found an existing agent, or explicit agent_version was supplied). We have no creation-time baseline…'

Automated review by giles17's agents